Skip to content

fix(plugin-auth)!: clearing the allowed IP ranges applies at once, and a console save that leaves out the saving address is refused - #30309

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-2etnqx
Nov 5, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/project-thread-2etnqx

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Bill · project thread

domain:server#2 · os-bill · session_01Utp6Ca3N2EsTAqg7jhabeL

Fixes #30203

Clause-②: yes (narrowing) — a settings console save (PUT /api/settings/auth) of an allowed_ip_ranges list none of whose entries matches the saving request's own address is refused, where it was stored before; and @objectstack/service-settings widens its published surface: three new exported types (SettingsWriteOptions, SettingsRequestTransport, SettingsWritePredicateOptions), setMany's fourth parameter accepts SettingsWriteOptions, registerWritePredicate takes an optional fourth options, and SettingsWritePredicateInput gains an optional transport.
Metadata-protocol: none
Skills-docs: none — skills/** teaches no allowed_ip_ranges; content/docs/permissions/authentication.mdx describes the setting and does not yet state the save refusal or the recovery, which ship here in the refusal text and this entry (the page is outside this change's surface and is named for a follow-up).

维护者速读

  • 改了什么:「允许的 IP 段」清空或恢复默认后立即生效,登录不再被旧列表继续拒绝到重启;在设置页保存一份没有任何一项匹配自己当前地址的列表时,保存被拒,提示里写明你的地址,以及被锁在外面时的救援办法(把环境变量 OS_AUTH_ALLOWED_IP_RANGES 设为空值后重启)。
  • 为什么改:卡上实测,管理员在家里存了办公室网段,没有任何提示,下次登录才发现进不来;清空字段后页面说已保存,登录却一直 403,只有重启服务器才恢复,业务管理员做不到也看不出原因。
  • 风险与回滚:收窄。只影响经设置页(HTTP)保存一份不匹配保存者自己地址的列表这一种操作;能匹配自己地址的列表、清空、启动与种子等进程内写入都不变。判的地址与登录闸门判的是同一个(含 server.trustProxy 规则)。回滚即撤本 PR。
  • 先看:packages/plugins/plugin-auth/src/allowed-ip-ranges.ts(保存时的判定与四种语言的拒绝文案);packages/plugins/plugin-auth/src/auth-plugin.ts 的 allowed_ip_ranges 分支(清空时交回部署自己声明的列表);packages/services/service-settings/src/settings-routes.ts 的保存路由(把请求的连接交给写谓词,每个谓词只拿到它注册时点名的请求头)。
  • 两分钟验证:pnpm --filter @objectstack/plugin-auth exec vitest run src/allowed-ip-ranges.pin.test.ts,预期 1 个文件、9 条全绿。
  • 需要你做的:无,席位落地。

What changes

The rule: an allowed-IP list cannot lock out the administrator who saves it, and clearing it applies at once.

  • Clearing applies at once (@objectstack/plugin-auth). The console sends an emptied field as null. The settings cascade then resolves the key with source default, and applySettings skipped every default-sourced key, so the running allowedIpRanges kept the last stored list until a restart. Measured on the real SettingsService with the shipped auth manifest: after a null save and after resetNamespace('auth'), allowed_ip_ranges reads { value: null, source: 'default' }. Now a default-sourced value hands the gate back to the deployment's own declaration in the same pass: no list when it declared none, the allowedIpRanges the plugin options or an auth:configure handler declared otherwise (captured before the manager is built). The reset is applied only when the running list came from a stored setting, so a deployment that never stored one sees no extra config patch. isExplicit itself is unchanged.
  • The save is refused (@objectstack/plugin-auth). A settings write predicate on auth.allowed_ip_ranges (allowed-ip-ranges.ts) refuses a list none of whose entries matches the saving request's address: 400 SETTINGS_VALIDATION, invalid_value at the key, constraint: { clientAddress }, and a message in the caller's language (en, zh-CN, ja-JP, es-ES, the settings bundle's locales) that names the address and the recovery. The address is resolved by the auth manager's own resolveClientAddress (peer address, or the forwarded one under server.trustProxy) and matched by the gate's own ipMatchesRange, so the save judges the address the gate later judges. When the address cannot be established, every non-empty list is refused, because the gate refuses that request too. A clear is never refused; an in-process write carries no request and is not judged. The textarea parsing moved into parseAllowedIpRanges, shared by the binding and the predicate.
  • The save door carries the request (@objectstack/service-settings). setMany's options become SettingsWriteOptions (SettingsDisplayOptions plus an optional transport: the transport's peer address and the request headers), and a write predicate's input gains the same optional transport. The PUT /api/settings/:namespace route sets it. It rides the options rather than the caller's context, so the service still receives the resolver's very context (the two door pins that assert identity stay as they are). A predicate sees only the headers it names at registration: registerWritePredicate(namespace, key, predicate, { requestHeaders }) (SettingsWritePredicateOptions), narrowed per predicate in the service, so cookie, authorization or an API key never reaches one that did not ask for it. The allow-list predicate names the two forwarded-address headers the address rule reads (FORWARDED_CLIENT_ADDRESS_HEADERS, the rule's own list). No packages/spec change.
  • Recovery when already locked out. Every auth key already has a settings env override; OS_AUTH_ALLOWED_IP_RANGES set to an empty value is in force (source: 'env'), wins over the stored list, and opens the gate on the next boot. The refusal text and the changeset name it. No new env var.

Tests

packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts (9 cases), on the real AuthPlugin, the real SettingsService with the shipped auth manifest, and the real settings routes:

  • clearing (null) opens the gate in the same pass; restoring the namespace defaults does too; a clear returns to the deployment's own declared list, not to none;
  • a list without the peer address is refused (status, code, field code, constraint.clientAddress) and nothing is stored; a list that includes it saves and is enforced (preservation);
  • under trustProxy the forwarded address is judged, and without it the peer address, both ways;
  • an address the server cannot establish refuses any list; a clear is never refused and an in-process write is not judged;
  • OS_AUTH_ALLOWED_IP_RANGES='' opens the gate over a stored list on the next boot.

packages/services/service-settings/src/settings-write-predicate-transport.test.ts (3 cases), on the real service and routes: a predicate that named the forwarded-address headers gets exactly those plus the peer address and no cookie / authorization / x-api-key; one that named none gets the peer address and no header; an in-process write carries no transport.

Suites and typecheck (head 8767d6a)
  • pnpm --filter @objectstack/plugin-auth test: Test Files 221 passed (221), Tests 3885 passed | 10 skipped (3895).
  • pnpm --filter @objectstack/service-settings test: Test Files 68 passed (68), Tests 1162 passed (1162).
  • pnpm --filter @objectstack/plugin-auth typecheck: exit 0. pnpm --filter @objectstack/service-settings typecheck: exit 0.
  • First run of the service-settings suite went red on two door pins (settings-routes.anonymous-floor.pin.test.ts, settings-routes.auth-gate.pin.test.ts: "the service sees that very context", toBe identity) while the transport rode the context; moving it onto the write options turned both green with the pins unchanged.
Ablations (each put the forbidden behaviour back through `scripts/ablation-replace.mjs`, saw red, restored; the test resolves both packages from source, so no build sits between)
  • A. the clear branch disabled (} else if (false /* ABLATION-30203-A */ && …): 3 failed | 6 passed — the three clearing cases.
  • B. the predicate registration disabled: 3 failed | 6 passed — the peer, trustProxy and undetermined refusal cases.
  • C. the route's transport renamed away: 3 failed | 6 passed — the same three.
  • D. the service's forwarding of transport to the predicate deleted: 3 failed | 6 passed — the same three.
  • After all four (head 4a97112): git diff HEAD empty; each restore's blob equals the HEAD blob.
  • E (head 8767d6a). The allow-list predicate registered with requestHeaders: []: 1 failed | 8 passed — the trustProxy case (the forwarded address no longer reaches the predicate).
  • F (head 8767d6a). narrowTransport passing every header: in settings-write-predicate-transport.test.ts, 2 failed | 1 passed — both header cases. After E and F: git diff HEAD empty; each restore's blob equals the HEAD blob.

Acceptance notes

  • The same clear-skips bug in other auth keys, not fixed here. Every if (isExplicit(key)) branch in applySettings without a reset keeps the last stored value after a clear or a restore-default. Measured on the real composition: lockout_threshold 3 and max_concurrent_sessions_per_user 2 stay in force after both are saved null. A general fix needs each key's "deployment declared" value restored, across the refusable emailAndPassword pieces that Organization require-MFA can be turned on with two-factor off, and after the grace period everyone is locked out with no way to enroll #30201 and Authentication settings: turning off email and password with no other sign-in method saves without warning, and everyone is locked out #30202 are editing, so this card fixes allowed_ip_ranges only and names the family for the seat.
  • The console half. The field does not show the caller's current address before saving; the refusal carries it (constraint.clientAddress) for the console to show. That is the domain:ui half, named for the seat.
  • IPv6 ranges. ipMatchesRange matches IPv4 CIDR and exact addresses only, so an IPv6 CIDR entry matches nothing at either door; the refusal therefore says no entry matches the address rather than that the list leaves it out. The gap is the seat's to file.
  • The docs half. content/docs/permissions/authentication.mdx does not state the save refusal or the recovery override; outside this card's surface, named for the seat.

Verification

Gates and lint on head 8767d6aa (the contract-review round, origin/main merged in at 89ff3aa9); the dogfood reading is from 89ff3aa9.

Derived gates: 65 derived, 64 exit 0, 1 NOT MEASURED
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 65 commands for the 9 changed paths; each ran with its exit code captured before any pipe.
  • --ran reconciliation: "65 derived famil(ies) accounted for — 64 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)".
  • NOT MEASURED: pnpm check:dual-build-cjs-loads exits 3 (PREREQUISITE NOT MET: 35 packages without dist/; it needs a whole-workspace build). CI runs it.
  • node scripts/check-plugin-teardown-shape.mjs --self-test: exit 0 ("48 cases pass"; on the first round it needed git fetch origin 621a4876… on the shallow checkout).
  • pnpm lint (eslint . --no-inline-config): exit 0.
Full dogfood (owed by the (narrowing) arm, head 89ff3aa): 1 red, from main, not from this change

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic (untouched).
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes.
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: n/a.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the 维护者速读 block is Chinese by the Communication rule).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Utp6Ca3N2EsTAqg7jhabeL

…that excludes the caller is refused

A cleared or restored-default `allowed_ip_ranges` resolves with source
`default`, which the settings binding skipped, so the last stored list
stayed in force until a restart. It now hands the gate back to the
deployment's own declaration in the same pass.

A console save of a list that leaves out the saving request's own
address (resolved by the gate's own rule) is refused, naming the address
and the `OS_AUTH_ALLOWED_IP_RANGES` recovery. The settings routes pass
the request's transport to write predicates for it.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6Ca3N2EsTAqg7jhabeL
…tions, not on the caller's context

The door hands the service the resolver's very context; the request a
write arrived on rides SettingsWriteOptions.transport to the predicate.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6Ca3N2EsTAqg7jhabeL
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface labels Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-auth, @objectstack/service-settings, touching 31 documentable anchor(s).

14 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via SettingsService (symbol, a top-level class))
  • content/docs/deployment/cli.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/deployment/self-hosting.mdx (via AuthPlugin (symbol, a top-level class), COPY (symbol, a top-level const object))
  • content/docs/deployment/tenancy-modes.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/kernel/runtime-services/settings-service.mdx (via setMany (symbol, a method of class SettingsService))
  • content/docs/kernel/services-checklist.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthPlugin (symbol, a top-level class), allowed_ip_ranges (literal, a string literal in bindAuthSettings))
  • content/docs/permissions/authorization.mdx (via allowed_ip_ranges (literal, a string literal in bindAuthSettings))
  • content/docs/permissions/record-view-auditing.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/index.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/protocol/kernel/config-resolution.mdx (via SettingsService (symbol, a top-level class), registerSettingsRoutes (symbol, a top-level function), setMany (symbol, a method of class SettingsService))
  • content/docs/protocol/kernel/index.mdx (via SettingsService (symbol, a top-level class))
  • content/docs/upgrading.mdx (via AuthPlugin (symbol, a top-level class))

⛔ 8 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via setMany (symbol, a method of class SettingsService))
  • content/docs/releases/v17/17-3.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-5.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v17/17-6.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-0.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-1.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/18-2.mdx (via AuthPlugin (symbol, a top-level class))
  • content/docs/releases/v18/index.mdx (via AuthPlugin (symbol, a top-level class))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 8 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 103 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c1240a38ac2c23c8d526e476a20edeabbbefd6f6 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from f13607b3d76935a7595fbeeefa033155b4a4b8b1 — the merge of head 8767d6aa81725636037589b29b3ad59a9b4a37e9 into base c1240a38ac2c23c8d526e476a20edeabbbefd6f6, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f13607b3d76935a7595fbeeefa033155b4a4b8b1 && git checkout f13607b3d76935a7595fbeeefa033155b4a4b8b1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c1240a38ac2c23c8d526e476a20edeabbbefd6f6 8767d6aa81725636037589b29b3ad59a9b4a37e9 && git checkout -B drift-repro c1240a38ac2c23c8d526e476a20edeabbbefd6f6 && git merge --no-ff 8767d6aa81725636037589b29b3ad59a9b4a37e9

node scripts/docs-audit/affected-docs.mjs --json c1240a38ac2c23c8d526e476a20edeabbbefd6f6

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs c1240a38ac2c23c8d526e476a20edeabbbefd6f6 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-bill · session_01Utp6Cbvcmv8YWFhFJwzjKL

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 89ff3aa9e1d941b2a20ed3c33ebc13c81ea18f55
Local-runs: none
Scope: packages/plugins/plugin-auth/src/allowed-ip-ranges.ts, packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts, packages/plugins/plugin-auth/src/auth-plugin.ts, packages/services/service-settings/src/index.ts, packages/services/service-settings/src/settings-routes.ts, packages/services/service-settings/src/settings-service.ts, packages/services/service-settings/src/settings-service.types.ts, .changeset/30203-allowed-ip-ranges-live.md + packages/plugins/plugin-auth/src/client-address.ts (the trust rule), packages/plugins/plugin-auth/src/auth-manager.ts (ipMatchesRange, unmapIpv4, isClientIpAllowed, resolveClientAddress, applyConfigPatch), the sign-in gate and the per-piece application in auth-plugin.ts (the registerAuthRoutes D5 block and applyPiece), packages/services/service-settings/src/manifests/auth.manifest.ts (allowed_ip_ranges, scope global), effectiveEnvOverride / coerceEnvValue / registerWritePredicate / setMany pre-flight in settings-service.ts, packages/plugins/plugin-hono-server/src/adapter.ts (readRemoteAddress), packages/spec/src/contracts/http-server.ts (remoteAddress), docs/adr/0069-enterprise-authentication-hardening.md D5, scripts/pm/clause2-line.mjs and scripts/check-changeset-no-major.mjs (for ②), and PR #30298 at 249ec4c4 (for ③). Inputs: card #30203 body and comments 6648444250, 6649086146, 6650755695; PR body, file list, net diff origin/main...89ff3aa9 (8 files, +507 / -12); the head's check-runs.

① Derived judgments

  • Which doors are judged — right. The predicate runs inside SettingsService.setMany (refuseByWritePredicates, after the declared constraints, before any row) and judges only when the input carries transport. The only producer of transport is the PUT /api/settings/:namespace route, which always sets it (headers always; remoteAddress when the adapter has one). So: the console save is judged; an in-process set / setMany (boot, seed, migration, an auth:configure host, resetNamespace) carries no request and is let through; a null (clear) returns before any address is read. A boot or non-request write must be neither refused nor skipped: there is no caller connection to lock out, and refusing one would brick a seeded deployment. Correct. A request write that is not the console — an IaC runner or a CLI calling the REST door from a host outside the list — IS refused; that is the rule applied honestly (the gate would refuse that host too), and the refusal names the recovery.
  • Clearing semantics — right. declaredAllowedIpRanges is captured at init after ctx.trigger('auth:configure', authConfig) (so a host handler's list counts), and the else if (this.allowedIpRangesFromSettings) branch only emits a patch when the running list came from a stored setting, so a deployment that never stored one sees no extra patch. The reset rides the non-refusable patch piece, applyConfigPatch spreads allowedIpRanges whole, and isClientIpAllowed reads an empty array as open. A cleared value with a deployment-declared list returns to that list, not to none (pinned). isExplicit is untouched, so the sibling keys keep the old behaviour, reported in ③.
  • The env pin — right. OS_AUTH_ALLOWED_IP_RANGES='' is a string in this.env, coerceEnvValue hands it back unchanged for a textarea, the key resolves source: 'env', isExplicit is true, parseAllowedIpRanges('') is [], the gate opens (pinned). While pinned the key is locked, so the console save is refused 409 SETTINGS_LOCKED in the pre-flight, before any predicate.
  • The new public surface — the declaration is WRONG. @objectstack/service-settings is a published package (private unset, 18.3.0). This head adds two exports to its index.ts (SettingsWriteOptions, SettingsRequestTransport), widens setMany's fourth parameter from SettingsDisplayOptions to SettingsWriteOptions, and adds transport to SettingsWritePredicateInput, the input every predicate author reads. That is a purely additive widening of a published package's public surface — the question the Clause-②: line answers (扩大公开面吗) — and the dispatch itself said yes for new published code. The PR body and the changeset declare no (narrowing) and say "nothing is widened", while the same changeset's fourth bullet describes exactly this widening. The honest line is Clause-②: yes (narrowing): a diff that widens one surface (the settings write seam) and narrows another (the console save). Under ruling A the level is decoupled from this line, so patch stays correct with Metadata-protocol: none; the narrowing arm keeps this review at CONTRACT_REVIEW_TIER. Must-fix (see ②).

② Semver level

  • Changeset .changeset/30203-allowed-ip-ranges-live.md: @objectstack/plugin-auth: patch, @objectstack/service-settings: patch. Level right: Metadata-protocol: none takes patch; a non-protocol narrowing ships patch with its BREAKING banner (AGENTS.md Post-Task Checklist step 3, ruling A). The **BREAKING** banner is present and states who moves and how; the ADR-0087 marker is present, disposition not-required, category no-migration-prescription, and the body carries no FROM → TO prescription (the "Who moves" bullet is prose, no arrow and no table), so the category holds. Check Changeset and the lint gate that runs the ADR-0087 registration are green on this head.
  • Declaration lines: Clause-②: and Metadata-protocol: are byte-identical between the PR body and the changeset. Both are wrong in the same way (① last bullet). Must-fix: rewrite the line in BOTH places to Clause-②: yes (narrowing) with reasoning that names the widened surface (the two new service-settings exports, the setMany options and the predicate input) and the narrowed one (the console save); keep Metadata-protocol: none and both patch levels. A yes with none and patch is the shape ruling A made legal; nothing else in the changeset needs to move.

③ Boundary flags

④ Design judgment

Judged on the tree, against the sign-in gate and the settings service as cited.

  • Same address on both doors — holds. The sign-in gate resolves mgr.resolveClientAddress({ headers, ...transportOf(c) }) and judges isClientIpAllowed; the predicate resolves mgr.resolveClientAddress({ headers, remoteAddress }) from the route's req.headers / req.remoteAddress and judges ipMatchesRange. Both read trustProxy live from the manager's config, both read the same forwarded-header list (client-address.ts, leftmost X-Forwarded-For, then X-Real-IP, believed only under trustProxy), and the hono adapter fills IHttpRequest.remoteAddress from the same socket readRemoteAddress reads. trustProxy off: the proxy's address is judged at both doors (so an admin behind an untrusted proxy saving the office range is refused, naming the proxy address — correct, that is the deployment's bug and the gate would refuse the same). trustProxy on: the forwarded client address at both. Pinned both ways.
  • Undetermined address — holds. resolveClientAddress returns undefined when neither fact exists; the gate refuses such a request while any list is set; the predicate refuses any non-empty list with clientAddress: null and the undetermined copy. Consistent; pinned.
  • IPv4-mapped / IPv6. unmapIpv4 folds ::ffff:a.b.c.d on both sides, so a dual-stack peer saves and signs in against an IPv4 range (pinned with ::ffff:127.0.0.1). Plain IPv6 matches exact-only on both doors, and an IPv6 CIDR never matches anywhere (ipMatchesRange falls back to exact match when either side is not IPv4) — a pre-existing gate limitation, now consistently applied at the save. Should-fix: the outside copy says the list "does not include" the address; for an admin at 2001:db8::1 saving 2001:db8::/32 the list does include it textually and the matcher cannot read it. Word it as "does not match" or name the matcher's reach (IPv4 CIDR and exact addresses), and file the IPv6-CIDR gap as a card against the gate (the manifest description promises "CIDR ranges" unqualified).
  • CIDR parsing errors — no crash path. parseAllowedIpRanges splits and trims only; ipMatchesRange never throws (/33, a non-numeric prefix or a hostname fall to exact match). A malformed-only list is refused at the save because nothing can match the caller — a protective side effect, not validation; a mixed list saves with the malformed entry inert, as before. Pre-existing; no finding against this PR.
  • Non-HTTP contexts — allowed, by construction. No transport ⇒ undefined before any read; no authManager yet ⇒ undefined. Nothing crashes. The only in-tree in-process writers of settings outside this package write other keys. Right.
  • Env-pinned key and restore-default. Covered in ①: a locked key refuses 409 before predicates; resetNamespace and a null save both resolve default, fire the subscription, and the else-branch returns the gate to the declared list (pinned).
  • Forging transport — not reachable. transport is the route's fourth argument to setMany, never read from the body; a body key transport (or values.transport) is an unknown key and the pre-flight refuses the batch 400 UNKNOWN_KEY. A spoofed X-Forwarded-For is believed only under trustProxy, exactly as the gate believes it; the caller already holds manage_platform_settings, so the check is a self-lockout belt, not an authorization control — and it is spelled so in the types. Right.
  • Should-fix (information flow): SettingsRequestTransport.headers hands the ENTIRE header map — cookie, authorization, API keys — to every write predicate any plugin registers, while the only consumer reads the forwarded-address family. Narrow what the route forwards to the forwarded-address headers (or document in the TSDoc that a predicate receives credential-bearing headers and must not log or store them). The current TSDoc says the transport "is never stored or logged"; nothing enforces that on a predicate author.
  • Should-fix (nit): SettingsReadSurface.registerWritePredicate? takes its input as any; unknown with the structural AllowedIpRangesWriteInput pick keeps plugin-auth free of a service-settings type dependency without an any.
  • Tests: the pin boots the real AuthPlugin, the real SettingsService with the shipped auth manifest and the real settings routes through the vitest alias; @objectstack/service-settings is a workspace devDependency of plugin-auth, so the file runs in Test Core. Nine cases cover each branch the four ablations name. Good.
  • Degradation levels: the predicate-registration catch logs warn (functional: the belt is missing, the gate still enforces) — right level.

Check-runs on 89ff3aa9 (polled every two minutes from 05:47 to 05:57 UTC, the ten-minute cap): green — Lint & Repo Gates and every Lint · lane, Governed Surface Queue Guard, Check Changeset, Check PR Size, Dogfood Regression Gate, Test Core (2/8, 5/8, 6/8, 7/8, 8/8), Type Check · source gates, Type Check · debt ledger, Type Check · consumer gates, Type Check · workspace, Build Core, Test UI, Live E2E UI, the PR-body guards. Still in_progress at the last poll: Test Core (1/8, 3/8, 4/8). No red on this head; judged with what is there. The verdict below does not depend on them: it is decided by the declaration defect in ②.

Findings

  • MUST-FIX (①/②): Clause-②: no (narrowing) is false on the tree — @objectstack/service-settings gains two published exports, a widened setMany options type and a new predicate-input field; declare Clause-②: yes (narrowing) identically in the PR body and the changeset, levels and Metadata-protocol: none unchanged.
  • SHOULD-FIX (④): forward only the forwarded-address headers in SettingsRequestTransport, or state in its TSDoc that predicates receive credential-bearing headers.
  • SHOULD-FIX (④): the outside refusal copy should say "does not match" (or name the matcher's IPv4-CIDR / exact reach); file the IPv6-CIDR gap on the gate as a card.
  • SHOULD-FIX (④, nit): registerWritePredicate? on SettingsReadSurface takes unknown, not any.
  • SEAT (③): file the clear-skips family and the fix(plugin-security): every member reads and posts comments, and reads the activity history, of the records they can read #30222 dogfood red as cards; name the docs and console halves as cards.

Implemented-by: session_01Utp6Ca3N2EsTAqg7jhabeL
Reviewed-by: session_01Utp6Cbvcmv8YWFhFJwzjKL

VERDICT: FAIL

Record taken 2026-11-05T05:57Z.

…quest headers it names; the refusal says no entry matches

Contract review 6650936040: the route handed every predicate the whole
header table. registerWritePredicate now takes requestHeaders, and the
allow-list predicate names the forwarded-address headers alone. The
refusal says no entry matches the address (an IPv6 CIDR entry matches
nothing today), and the settings slot's predicate input is unknown.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Utp6Ca3N2EsTAqg7jhabeL
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-bill · session_01Utp6Cbvcmv8YWFhFJwzjKL

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 8767d6aa81725636037589b29b3ad59a9b4a37e9
Local-runs: none
Scope: packages/plugins/plugin-auth/src/allowed-ip-ranges.ts, packages/plugins/plugin-auth/src/auth-plugin.ts, packages/services/service-settings/src/index.ts, packages/services/service-settings/src/settings-routes.ts, packages/services/service-settings/src/settings-service.ts, packages/services/service-settings/src/settings-service.types.ts, packages/services/service-settings/src/settings-write-predicate-transport.test.ts, .changeset/30203-allowed-ip-ranges-live.md + packages/plugins/plugin-auth/src/client-address.ts (resolveClientAddress, firstHeader), packages/plugins/plugin-auth/src/client-address-forwarding.ts (FORWARDED_CLIENT_ADDRESS_HEADERS), packages/plugins/plugin-auth/src/client-address-vendor.ts (the vendored reader's one header), packages/plugins/plugin-auth/src/auth-manager.ts (resolveClientAddress, ipMatchesRange, isClientIpAllowed), the D5 gate block and transportOf in auth-plugin.ts, packages/plugins/plugin-auth/src/allowed-ip-ranges.pin.test.ts (unchanged by the delta; the trustProxy case), packages/plugins/plugin-hono-server/src/adapter.ts (rawHeaders = c.req.header(), readRemoteAddress), packages/spec/src/contracts/http-server.ts (IHttpRequest.headers, remoteAddress), packages/services/service-settings/src/localization-currency-lock.ts and settings-write-predicate.test.ts (the other registerWritePredicate callers), packages/plugins/plugin-auth/src/index.ts and package.json (whether the new module is republished), PR #30298 at 249ec4c4 (for ③), cards #30320 and #30255 (for ③). Inputs: card #30203 body and comments 6648444250, 6649086146, 6650755695, 6650829184; PR #30309 body as replaced at 06:58 UTC, file list (9 files, +679 / -18 against main), the net diff origin/main...8767d6aa, the delta 89ff3aa9..8767d6aa (one commit, 8 files, +191 / -25; no main merge in the range); record 6650936040; the check-runs on this head. This record reviews the DELTA against record 6650936040; the judgments of that record on the unchanged parts stand and are not restated.

① Derived judgments

  • The delta, hunk by hunk. (a) settings-service.ts: the predicate registry now stores { predicate, requestHeaders } per key, registerWritePredicate takes an optional fourth options whose requestHeaders are lower-cased into a Set at registration, and refuseByWritePredicates hands each predicate narrowTransport(display.transport, registered.requestHeaders) instead of the whole transport. narrowTransport copies only the header entries whose lower-cased name is in the set, under their incoming spelling, plus remoteAddress when present; it builds a fresh object per predicate, so no predicate can mutate what a sibling sees. (b) settings-service.types.ts: new SettingsWritePredicateOptions, and SettingsWritePredicateInput.transport's TSDoc now states the narrowing. (c) index.ts: SettingsWritePredicateOptions exported. (d) settings-routes.ts: comment only; the route still hands the full req.headers and req.remoteAddress to the service, which is right: narrowing per predicate has to happen where the registrations are known. (e) auth-plugin.ts: the structural SettingsReadSurface.registerWritePredicate? takes input: unknown and an optional options, and the registration names FORWARDED_CLIENT_ADDRESS_HEADERS. (f) allowed-ip-ranges.ts: the predicate's parameter is unknown, read structurally through AllowedIpRangesWriteInput; the outside copy in all four locales says no entry matches. (g) the new settings-write-predicate-transport.test.ts, three cases. (h) the changeset's declaration and prose. Every accept-set change of the delta is a narrowing of what a predicate receives (fewer headers), not of what an administrator can save; the save's accept set is unchanged from 89ff3aa9.
  • The declaration is now true and complete. Against the net diff of @objectstack/service-settings (published, 18.3.0) the public surface gains exactly: the exported types SettingsRequestTransport, SettingsWriteOptions, SettingsWritePredicateOptions; setMany's fourth parameter typed SettingsWriteOptions in place of SettingsDisplayOptions; registerWritePredicate's optional fourth options; SettingsWritePredicateInput.transport. The Clause-②: yes (narrowing) line names each of these six and nothing that is not there. @objectstack/plugin-auth (published, 18.3.0) adds allowed-ip-ranges.ts, which neither its index.ts nor its exports map republishes, so no unnamed widening hides there. The narrowing half of the line (a console save none of whose entries matches the saving address is refused where it was stored before) is the behaviour 89ff3aa9 already carried and record 6650936040 judged right. The must-fix of that record is closed.
  • Callers that predate the fourth parameter still fit. localization-currency-lock.ts declares a host interface with the three-parameter registerWritePredicate and calls it with three; settings-write-predicate.test.ts calls it with three. The parameter defaults to {}, a currency-lock predicate receives headers: {} plus the peer address on a request write, and it reads neither. Right.

② Semver level

  • Changeset .changeset/30203-allowed-ip-ranges-live.md: @objectstack/plugin-auth: patch, @objectstack/service-settings: patch; Metadata-protocol: none, so patch is the level ruling A assigns; a non-protocol narrowing ships patch with its banner. The **BREAKING** banner is present and names who moves; the ADR-0087 marker is present, exactly one, disposition not-required, category no-migration-prescription, and the body still carries no FROM → TO prescription, so the category holds. Check Changeset and Lint & Repo Gates (which runs the ADR-0087 registration gate) are green on this head.
  • Declaration lines: Clause-②:, Metadata-protocol: and Skills-docs: are byte-identical between the PR body (as replaced at 06:58 UTC) and the changeset (sha256 of each line compared). Clause-②: yes (narrowing) is a well-formed arm of the closed pair. Nothing to fix here.

③ Boundary flags

④ Design judgment

Judged on the tree, trying to break narrowTransport and the per-predicate header contract against the gate it must agree with.

  • Header-name case — holds. Registration lower-cases every name (new Set(names.map(toLowerCase))); narrowTransport compares name.toLowerCase() of each incoming key against that set and keeps the incoming spelling. So requestHeaders: ['X-Forwarded-For', 'x-real-ip'] against lower-case incoming keys matches (pinned in the new test, first case). Both doors take their header map from Hono's c.req.header(): the gate directly (D5 block, auth-plugin.ts), the save through the hono adapter's rawHeaders = c.req.header() into IHttpRequest.headers; Hono lower-cases keys, and the list is lower-case, so every lookup lines up. The one spelling that would not match is an adapter delivering mixed-case keys: narrowTransport would still pass them through (it matches case-insensitively), but resolveClientAddress's firstHeader reads headers[name] ?? headers[name.toLowerCase()] with a lower-case name, so it would miss them at the gate and at the save alike. Same answer at both doors is what the rule requires; no finding against this delta.
  • Coverage of the forwarded headers — complete by construction. resolveClientAddress reads forwarded headers only inside for (const name of FORWARDED_CLIENT_ADDRESS_HEADERS), and the plugin registers that very constant, so the predicate receives every header the address rule can read (x-forwarded-for, x-real-ip) and no other. No Forwarded, CF-Connecting-IP or True-Client-IP reader exists in plugin-auth, the runtime or the hono adapter (grepped); the vendored library reads only the platform-written x-objectstack-client-address, which the platform sets from the already-resolved address, never from the client. If a later change widens the rule's list, the registration widens with it, because it names the list and not two literals. The save and the gate therefore judge the same address under trustProxy on and off. Ablation E in the body (registering []) reds exactly the trustProxy case, which is the failure shape a missing header would produce; the pin would catch a drift.
  • A predicate with no requestHeaders — receives headers: {} and the peer address (pinned, second case). This is the right default: the peer address is the one fact a client cannot forge, and a predicate that wants a header says so where its registration can be audited. The currency-lock predicate is such a one and reads neither.
  • Credential headers — do not reach the allow-list predicate (pinned, first case: cookie, authorization, x-api-key present on the request, absent from the narrowed transport). Ablation F (pass every header) reds both header cases. The TSDoc on SettingsWritePredicateOptions.requestHeaders says "lower-case"; the matching is case-insensitive, so the doc under-promises harmlessly. No action.
  • In-process writes — unchanged: display.transport absent ⇒ no transport key on the input at all (pinned, third case); narrowTransport is never reached.
  • input: unknown — the predicate reads (input ?? {}) as AllowedIpRangesWriteInput and then guards transport, next (through parseAllowedIpRanges's typeof check) and manager(); a malformed input yields undefined, never a throw. The structural pick keeps plugin-auth free of a service-settings type import, as the prior record asked.
  • Refusal copy — "No entry in this list matches …" is true for the IPv6-CIDR case the prior record described (an entry textually containing the address that the matcher cannot read) and for the plain case; the recovery sentence is unchanged. Should-fix (nit, non-blocking): the changeset's **BREAKING** banner and the PR title still say the list "does not include" / "leaves out" the saving address; the upgrading administrator reads the banner in CHANGELOG.md with the same IPv6 imprecision the copy fix removed. One phrase: "that no entry of which matches the address they are connected from".
  • Tests: the new file boots the real SettingsService with the shipped branding manifest and the real settings routes, and asserts the narrowed transport with toEqual (an exact shape, so an extra header would fail it). Good.

Check-runs on 8767d6aa (polled at 07:00 and 07:02 UTC): 52 runs, 37 success, 15 skipped (the path-filtered UI lanes and opt-in harnesses), 0 in_progress, 0 failure. Green: Lint & Repo Gates and every Lint · lane, TypeScript Type Check and its four lanes, Test Core and all eight shards, Build Core, Governed Surface Queue Guard, Check Changeset, Check PR Size, Dogfood Regression Gate, Test UI, Live E2E UI, the PR-body guards. No red on this head.

Findings

Implemented-by: session_01Utp6Ca3N2EsTAqg7jhabeL
Reviewed-by: session_01Utp6Cbvcmv8YWFhFJwzjKL

VERDICT: PASS

Record taken 2026-11-05T07:05Z.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-bill · session_01Utp6Cbvcmv8YWFhFJwzjKL

维护者速读

  • 改了什么: 两件。① 清空「允许的 IP 段」后立即生效,运行中的名单退回部署自己声明的名单,不用再重启。② 在设置页保存一份不匹配保存者自己地址的名单时,直接拒绝(400 SETTINGS_VALIDATION),提示里写明对方的地址,以及已经锁在门外时的补救办法:把 OS_AUTH_ALLOWED_IP_RANGES 设成空。为此 service-settings 多了一条通道,把请求里的地址交给写入谓词:每个谓词只拿到自己登记要的头,cookie、authorization、API key 都到不了。
  • 为什么改: 原来管理员能把自己排除在外而毫无提示,清空字段也要重启才生效(Allowed IP ranges: an administrator can exclude their own address unwarned, and clearing the field does not take effect until a restart #30203)。
  • 风险与回滚: 只收窄一种保存:设置页上会排除保存者自己的名单(BREAKING,changeset 写明,带 ADR-0087 标记)。启动、种子数据、代码里的写入不受影响。回滚即撤本 PR。
  • 先看: packages/plugins/plugin-auth/src/allowed-ip-ranges.ts,再看 settings-service.ts 的 narrowTransport。
  • 两分钟验证: pnpm --filter @objectstack/plugin-auth exec vitest run src/allowed-ip-ranges.pin.test.ts。
  • 需要您做的: 无,席位经合并队列落地(非受管面,697 行)。
  • 席位意见: ACCEPT。契约复审第一轮 FAIL(6650936040,申报错),已改;增量复审 PASS(6651963356)。

席位核对 — ACCEPT · domain:server#2 · head 8767d6aa81725636037589b29b3ad59a9b4a37e9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling Fix lands in tooling, not a product package — enters pm:queue only with Unblocks: or a named surface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allowed IP ranges: an administrator can exclude their own address unwarned, and clearing the field does not take effect until a restart

1 participant